WCA Live Result Admin #1: Submit and Updating Results #10776
Conversation
| AddLiveResultJob.perform_now({ results: results, | ||
| round_id: round_id, | ||
| registration_id: registration_id, | ||
| entered_by: current_user }) |
There was a problem hiding this comment.
Regarding my other comment at #10685 (comment): You would also need to update the call here and get rid of the hash { braces.
| unless result.present? | ||
| return render json: { status: "result does not exist" }, status: :unprocessable_entity | ||
| end |
There was a problem hiding this comment.
I think this can be return render json: ... unless result.present?. Surprised that RuboCop isn't complaining.
| # TODO: What is the best way to do this? | ||
| r = Result.build({ value1: results[0], value2: results[1], value3: results[2], value4: results[3] || 0, value5: results[4] || 0, event_id: round.event.id, round_type_id: round.round_type_id, format_id: round.format_id }) | ||
|
|
||
| result.update(average: r.compute_correct_average, best: r.compute_correct_best, live_attempts: new_attempts, entered_at: Time.now.utc, entered_by: current_user) |
There was a problem hiding this comment.
Should we send a timestamp from the frontend when the query was fired?
If we're running this in a queue later, the "now" here in the backend could be significantly different from the "now" happening for the people in front of their screens.
There was a problem hiding this comment.
updating is currently not done in a queue, maybe it should? But we aren't doing that for registrations for example (but there updating doesn't create anything new)
| new_attempts = results.map.with_index(1) do |r, i| | ||
| same_result = previous_attempts.find_by(result: r, attempt_number: i) | ||
| if same_result.present? | ||
| same_result | ||
| else | ||
| different_result = previous_attempts.find_by(attempt_number: i) | ||
| new_result = LiveAttempt.build(result: r, attempt_number: i) | ||
| different_result&.update(replaced_by_id: new_result.id) | ||
| new_result | ||
| end | ||
| end |
There was a problem hiding this comment.
Note to self: I have a hunch there might be an even more efficient way to handle this.
There was a problem hiding this comment.
Will play around later when I have more headspace.
| def open_round | ||
| round_id = params.require(:round_id) | ||
| competition_id = params.require(:competition_id) | ||
| round = Round.find(round_id) | ||
|
|
||
| if round.is_open? | ||
| flash[:danger] = "Round is already open" | ||
| return redirect_to live_schedule_admin_path(competition_id: competition_id) | ||
| end | ||
|
|
||
| unless round.round_can_be_opened? | ||
| flash[:danger] = "You can't open this round yet" | ||
| return redirect_to live_schedule_admin_path(competition_id: competition_id) | ||
| end | ||
|
|
||
| round.update(is_open: true) | ||
| flash[:success] = "Successfully opened round" | ||
| redirect_to live_schedule_admin_path(competition_id: competition_id) | ||
| end |
There was a problem hiding this comment.
Isn't this part of a separate PR?
There was a problem hiding this comment.
oh yeah, that was accidental
| options={competitors.toSorted((a, b) => a.id - b.id).map((p) => ({ | ||
| key: p.id, | ||
| value: p.id, | ||
| registrationId: p.registration_id, | ||
| text: `${p.user.name} (${p.registration_id})`, | ||
| }))} |
There was a problem hiding this comment.
Same remark about pre-computing as above.
| text: `${p.user.name} (${p.registration_id})`, | ||
| }))} | ||
| /> | ||
| {Array.from(zeroedArrayOfSize(solveCount).keys()).map((index) => ( |
| } | ||
| }; | ||
|
|
||
| return ( |
There was a problem hiding this comment.
Just from the fact that I wrote a lot of "same as above" comments below: How much code is this actually sharing with DoubleCheck?
There was a problem hiding this comment.
this definitely can be combined
| return {}; | ||
| } | ||
| if (result.advancing) { | ||
| return { backgroundColor: 'rgb(0, 230, 118)' }; |
There was a problem hiding this comment.
The green tone should probably be stored somewhere, especially since you're reusing it with transparency again below
| export const recordTagStyle = (tag) => { | ||
| const styles = { | ||
| display: 'block', | ||
| lineHeight: 1, | ||
| padding: '0.3em 0.4em', | ||
| borderRadius: '4px', | ||
| fontWeight: 600, | ||
| fontSize: '0.6em', | ||
| position: 'absolute', | ||
| top: '0px', | ||
| right: '0px', | ||
| transform: 'translate(110%, -40%)', | ||
| color: 'rgb(255, 255, 255)', | ||
| }; | ||
|
|
||
| switch (tag) { | ||
| case 'WR': { | ||
| styles.backgroundColor = 'rgb(244, 67, 54)'; | ||
| break; | ||
| } | ||
| case 'CR': { | ||
| styles.backgroundColor = 'rgb(255, 235, 59)'; | ||
| break; | ||
| } | ||
| case 'NR': { | ||
| styles.backgroundColor = 'rgb(0, 230, 118)'; | ||
| break; | ||
| } | ||
| case 'PR': { | ||
| styles.backgroundColor = 'rgb(66, 66, 66)'; | ||
| break; | ||
| } | ||
| default: { | ||
| return {}; | ||
| } | ||
| } | ||
| return styles; | ||
| }; |
There was a problem hiding this comment.
Not a fan of big custom styles at all. SemUI has a Label component, can that work?
For specific colors, I'll agree to some CSS styling but display: block and others should definitely be left to SemUI.
There was a problem hiding this comment.
yes, this was a fairly lazy way of recreating the same thing as wca live
Simple Submitting/Updating Results and double checking them. Does not include calculating advancing/rankings/records. Those will be follow up PRs (which I have already done, but I want to add tests).
Have not checked if I deleted everything related to those features, will do after dinner.